Recently someone made this change: > -nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, > - nsIContent* aChild, > - nsIContent* aOldNextSibling, > - RemoveFlags aFlags, > - bool* aDidReconstruct, > +nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, > + nsIContent* aChild, > + nsIContent* aOldNextSibling, > + RemoveFlags aFlags, > + bool* aDidReconstruct, > nsIContent** aDestroyedFramesFor) and justified that with "clang-format ¯\_( ͡° ͜ʖ ͡°)_/¯". IMO, the above change is a regression in readability. The above is a fairly mild example since the first four names align naturally, but there are far worse examples. Can we make clang-format align the parameter names, please?

Comment on attachment 8883525 [details] Bug 1378250 - clang-format: Align consecutive declarations https://reviewboard.mozilla.org/r/154450/#review159536

Pushed by sledru@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b14cbbf5256a clang-format: Align consecutive declarations r=andi

Ugh, I think this is fine for arguments, but just tried this, and this makes code inside a function written as: nsStyleChangeList currentChanges(StyleBackendType::Servo); DocumentStyleRootIterator iter(doc); bool anyStyleChanged = false; To be formatted as: nsStyleChangeList currentChanges(StyleBackendType::Servo); DocumentStyleRootIterator iter(doc); bool anyStyleChanged = false; Which IMO is unheard-of in our codebase, and is quite ugly personally.

Also code like: ServoStyleSet* styleSet = StyleSet(); nsIDocument* doc = PresContext()->Document(); bool animationOnly = aRestyleBehavior == TraversalRestyleBehavior::ForAnimationOnly; Becomes: ServoStyleSet* styleSet = StyleSet(); nsIDocument* doc = PresContext()->Document(); bool animationOnly = aRestyleBehavior == TraversalRestyleBehavior::ForAnimationOnly; Which is unreadable IMO.

Indeed, this isn't great :/ Tomcat, could you backout my change? sorry :/

(In reply to Sylvestre Ledru [:sylvestre] from comment #6) > Indeed, this isn't great :/ > > Tomcat, could you backout my change? > sorry :/ np :) done in https://hg.mozilla.org/integration/autoland/rev/e1169c7413b7be542afa07053a1c81a5e293ffad

Should we wontfix this? While I understand Mats' concern in comment 0, there does not appear to be consensus on this preference. I imagine it's in the minority, but I can ping dev-platform and the code style module owners. Personally I find the blame and burden I take for realigning things every time I add a new longer param (or shorten a longer param) to be problematic (there's a similar argument for prefixing member lists with ':' and ','). If the tool is doing it for me, the main argument against is diff/blame bloat which I still find problematic. Also what should we do in the case of multi-line arguments with multiple params? > foo(bar* q, foobar* r, > bazbar* s, int t); does that become: > foo(bar* q, foobar* r, > bazbar* s, int t); Or maybe that's not a valid issue if we have a rule of one argument per line; I don't see that stated anywhere, but I might be missing something.

Per further discussion on the dev-platform mailing list [1] I'm going to wontfix this. [1] https://groups.google.com/forum/#!topic/mozilla.dev.platform/De36Q5r9ZKo

FWIW AlignConsecutiveDeclarations wasn't designed to only work on declarations inside functions' argument lists, so the results we saw here were the intended results.